-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(a11y) Make Dropdown more accessible #2333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -45,8 +45,8 @@ export default class Navigation extends React.Component { | |||
<Dropdown | |||
className="navigation__languages" | |||
items={[ | |||
{ title: 'English', url: 'https://webpack.js.org/' }, | |||
{ title: '中文', url: 'https://doc.webpack-china.org/' } | |||
{ lang: 'en', title: 'English', url: 'https://webpack.js.org/' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason of this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's my mistake. This shouldn't be there. Thanks for taking a look :)
- make it focusable - allow navigation using arrow keys - dismiss on escape
a0626b0
to
b630674
Compare
@@ -46,7 +46,7 @@ export default class Navigation extends React.Component { | |||
className="navigation__languages" | |||
items={[ | |||
{ title: 'English', url: 'https://webpack.js.org/' }, | |||
{ title: '中文', url: 'https://doc.webpack-china.org/' } | |||
{ lang: 'zh', title: '中文', url: 'https://doc.webpack-china.org/' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is fine to left English language with an empty lang
attribute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, because we have already set lang globally with , so we don't have to set it here.
Thank you! |
This is a rework of #2325